fix(oracle): handle bracketed proxy usernames in go-ora URL by Deeven-Seru · Pull Request #2468 · googleapis/mcp-toolbox

Deeven-Seru · 2026-02-13T06:23:06Z

Summary

build go-ora DSNs using encoded userinfo so Oracle proxy usernames like user[client] are valid in URL form
decode already-percent-encoded credentials before re-encoding to avoid double-encoding
keep connect string path (host:port/service) intact and preserve wallet query handling
add regression tests covering bracketed proxy users, pre-encoded users, and wallet/no-wallet forms

gemini-code-assist · 2026-02-13T06:23:20Z

Summary of Changes

Hello @DEVELOPER-DEEVEN, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses an issue where Oracle proxy usernames with special characters were not correctly handled when constructing go-ora connection strings. The changes introduce robust URL encoding for user credentials, prevent accidental double-encoding, and ensure proper handling of wallet configurations. This significantly improves the reliability of Oracle database connections, especially for complex user authentication setups.

Highlights

Oracle DSN Encoding: Implemented encoding for user information when building go-ora Data Source Names (DSNs) to correctly handle Oracle proxy usernames that contain special characters.
Prevent Double Encoding: Added logic to decode any already percent-encoded credentials before re-encoding them, preventing issues with double-encoding.
Connection String Preservation: Ensured that the connect string path remains intact and that wallet query handling is preserved during the DSN construction process.
Regression Tests: Introduced new regression tests to cover scenarios involving bracketed proxy users, pre-encoded users, and both wallet and non-wallet connection forms.

Changelog

internal/sources/oracle/oracle.go
- Imported the net/url package for URL manipulation.
- Added a new helper function buildGoOraConnString to centralize the logic for constructing go-ora connection strings, handling user info encoding and wallet parameters.
- Added a new helper function decodePercentEncodedUserInfo to safely decode URL-encoded strings, preventing double-encoding.
- Refactored the initOracleConnection function to utilize the new buildGoOraConnString for generating the final connection string, simplifying the connection setup logic.
internal/sources/oracle/oracle_connstring_test.go
- Added a new test file to provide comprehensive unit tests for the buildGoOraConnString function.
- Included test cases to verify correct encoding of credentials with special characters and wallet paths.
- Added a test case to confirm that already percent-encoded user information is not double-encoded.
- Included a test case for connection strings without a wallet.

Activity

No specific activity (comments, reviews, or progress updates) has been recorded for this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request addresses issues with special characters in Oracle usernames. The current connection string construction, however, is vulnerable to URL manipulation due to string concatenation, which could allow special characters like ? or # to bypass security controls such as SSL. It is recommended to use the net/url package for robust URL construction to mitigate this vulnerability and improve overall robustness against edge cases.

anubhav756 · 2026-02-17T14:14:15Z

/gcbrun

Yuan325 · 2026-02-18T19:24:26Z

Hi @DEVELOPER-DEEVEN, can you please rebase and clean the PR (seems like it's removing some already merged codes)? Thank you! :) Will take a look once this is ready

Deeven-Seru · 2026-02-19T04:56:29Z

@Yuan325 yuan PTAL I think it works now

Yuan325 · 2026-02-24T16:29:47Z

Hi @Deeven-Seru, seems like the rebasing is still removing some existing code (e.g. internal/log/log.go). Please double-check that only your intended updates made it into this PR. Thank you!

Deeven-Seru · 2026-03-03T15:36:01Z

/gemini review

gemini-code-assist

Code Review

This pull request correctly addresses the issue of handling special characters in Oracle proxy usernames by using net/url for proper encoding and includes decodePercentEncodedUserInfo to prevent double-encoding, along with comprehensive new unit tests in oracle_connstring_test.go. However, it introduces critical regressions and security vulnerabilities. The removal of the readOnly parameter and its associated DML handling logic in the RunSQL function breaks INSERT, UPDATE, and DELETE statements, which is also a significant security regression by removing a critical safety feature for executing SQL from untrusted sources. Furthermore, the new connection string construction logic is vulnerable to URL parameter injection and can produce malformed URLs if the base connection string already contains query parameters, and it breaks existing tests in oracle_test.go.

Deeven-Seru · 2026-03-03T16:05:38Z

Correction to prior note :

Addressed in latest force-push (6dfe250):

Restored RunSQL(ctx, statement, params, readOnly bool) and reintroduced the readOnly=false DML ExecContext path with rows_affected.
Updated go-ora DSN construction to preserve and merge existing query params safely when connectStringBase already contains a query, while still encoding userinfo and wallet params.
Added/expanded regression tests in internal/sources/oracle/oracle_connstring_test.go for bracketed proxy usernames, pre-encoded userinfo (no double-encoding), wallet/no-wallet, and existing-query-plus-wallet merge cases.

This is now a clean single-commit diff with only Oracle source/test changes. PTAL.

gemini-code-assist

Code Review

This pull request effectively addresses the issue of handling bracketed proxy usernames and other special characters in Oracle connection strings. The introduction of buildGoOraConnString and decodePercentEncodedUserInfo functions centralizes and robustly manages the construction of go-ora DSNs, preventing double-encoding and correctly integrating wallet locations and existing query parameters. The new test file oracle_connstring_test.go provides comprehensive coverage for these changes, significantly improving the reliability of Oracle connections.

Deeven-Seru · 2026-03-11T10:42:23Z

@Yuan325 ptal

Yuan325

Hi @Deeven-Seru, left some feedback on this, thank you for your contributions! I'll take another look once it's updated.

Deeven-Seru · 2026-03-17T04:22:53Z

colsing this as the issue isn fixed via #2469

Deeven-Seru requested a review from a team February 13, 2026 06:23

blunderbuss-gcf bot assigned Yuan325 Feb 13, 2026

gemini-code-assist bot reviewed Feb 13, 2026

View reviewed changes

Comment thread internal/sources/oracle/oracle.go Outdated

Deeven-Seru mentioned this pull request Feb 17, 2026

Oracle proxy username with brackets rejected in connection URL #2454

Closed

anubhav756 force-pushed the fix-2454-oracle-proxy-userinfo branch from 7642efa to 4233c3a Compare February 17, 2026 14:13

Deeven-Seru requested a review from a team February 18, 2026 04:36

Deeven-Seru force-pushed the fix-2454-oracle-proxy-userinfo branch from dd43c58 to ede7fdb Compare February 18, 2026 04:45

Deeven-Seru force-pushed the fix-2454-oracle-proxy-userinfo branch from ede7fdb to 687cb13 Compare February 19, 2026 04:46

Deeven-Seru force-pushed the fix-2454-oracle-proxy-userinfo branch from 7438b7a to 20051d4 Compare March 1, 2026 14:58

Deeven-Seru requested a review from a team as a code owner March 1, 2026 14:58

Deeven-Seru force-pushed the fix-2454-oracle-proxy-userinfo branch 2 times, most recently from 331dd0a to 41f7349 Compare March 2, 2026 15:24

gemini-code-assist bot reviewed Mar 3, 2026

View reviewed changes

Comment thread internal/sources/oracle/oracle.go Outdated

Comment thread internal/sources/oracle/oracle.go Outdated

Deeven-Seru force-pushed the fix-2454-oracle-proxy-userinfo branch from 6fe3cd5 to 6dfe250 Compare March 3, 2026 16:05

Deeven-Seru force-pushed the fix-2454-oracle-proxy-userinfo branch from 6dfe250 to 9bee2bf Compare March 3, 2026 16:06

gemini-code-assist bot reviewed Mar 3, 2026

View reviewed changes

Comment thread internal/sources/oracle/oracle.go Outdated

Comment thread internal/sources/oracle/oracle.go

Comment thread internal/sources/oracle/oracle.go Outdated

Comment thread internal/sources/oracle/oracle_connstring_test.go Outdated

Yuan325 requested changes Mar 12, 2026

View reviewed changes

Comment thread internal/sources/oracle/oracle.go Outdated

Comment thread internal/sources/oracle/oracle.go

Comment thread internal/sources/oracle/oracle.go

Comment thread internal/sources/oracle/oracle_connstring_test.go Outdated

Comment thread internal/sources/oracle/oracle.go

Deeven-Seru added 2 commits March 13, 2026 05:42

fix(oracle): preserve RunSQL readonly mode and query-safe DSN build

3f09a15

fix(oracle): clean go-ora connstring handling

49cf42f

Deeven-Seru force-pushed the fix-2454-oracle-proxy-userinfo branch from c1ed735 to 49cf42f Compare March 13, 2026 05:47

Deeven-Seru closed this Mar 17, 2026

Conversation

Deeven-Seru commented Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

gemini-code-assist bot commented Feb 13, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

anubhav756 commented Feb 17, 2026

Uh oh!

Yuan325 commented Feb 18, 2026

Uh oh!

Deeven-Seru commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Yuan325 commented Feb 24, 2026

Uh oh!

Deeven-Seru commented Mar 3, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Deeven-Seru commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Deeven-Seru commented Mar 11, 2026

Uh oh!

Yuan325 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Deeven-Seru commented Mar 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Deeven-Seru commented Feb 13, 2026 •

edited

Loading

Deeven-Seru commented Feb 19, 2026 •

edited

Loading

Deeven-Seru commented Mar 3, 2026 •

edited

Loading